-
Notifications
You must be signed in to change notification settings - Fork 803
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Csv import: GncDateFormat ->date_locale #2010
Conversation
90c022a
to
c3d797f
Compare
part2: save/restore: date-format -> locales
2c8ca3d
to
4cfa4a8
Compare
4cfa4a8
to
1ab2854
Compare
It didn't like Jun either. Does changing the locale change anything? |
not really! "Jun" "Jul" are also off. "September" is accepted. |
oops with en_US.utf8 "Sep":fail "Jun":fail "Jul":fail -- I guess it expects m/d/y |
libgnucash/engine/gnc-datetime.cpp
Outdated
if (U_FAILURE(status)) | ||
throw std::invalid_argument ("Cannot parse string"); | ||
|
||
tuple = std::make_tuple<DateFormatPtr, CalendarPtr>(std::move(formatter), std::move(calendar)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You forgot to add the tuple to the cache.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't: see the magic & in auto&
I did test via printfs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The std::move is slightly beyond my current understanding but it's a requirement for compilation to succeed. Chatgpt did suggest it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see the magic & in auto&
Very clever. I did miss that indeed.
The std::move is slightly beyond my current understanding
[std::tuple](http://en.cppreference.com/w/cpp/utility/tuple)<unwrap_decay_t<Types>...> make_tuple(Types&&... args)
In template context Types&&
means forwarding reference. It needs either a reference or an lvalue. A named variable is an rvalue so it needs to be cast into an lvalue. std::move
does that. std::move
is apparently confusing. Maybe the committee should have called it lvalue_cast
instead.
Seems it is based on poor information. I've never seen that month format in Australia, normally numeric or 3 character alpha. |
…parses test 500 dates in 797 locales filtered down to 796 locales in 3.37s
f1629d7
to
c164733
Compare
Ok let's kill this stinky hack in favour of #2011 |
Ok. For posterity, there was a bug in the last attempt, where you try two parsers. If the short parser failed, the medium parser would always fail as well because the Still all these parse attempts are costly. In the ideal case - where only a few parsers are valid - this is ok. However if I test on a string that's valid in many parsers the time to run all the tests add up quickly. For example with 500 date strings like '06/09/2021', over 600 parsers return a valid date, and the time it takes varies from 2,5 seconds to 3,5 seconds on my system. This can likely be optimized some more by eliminating failing short parsers from the cache. However I don't know if this "intelligent" guessing is really worth it in the end. |
offshoot #1710 -- here's attempt.
all GncDateFormat changed to use locales. The locale combobox is currently 799 items long!
but it doesn't seem to parse all dates. see example with debugging code. looks like ICU type issue which cannot understand "Sep" yet can understand "Apr".